Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#615: added TYPEFLAGS constants for TYPEATTR in ITypeInfo.GetTypeAttr() #619

Closed
wants to merge 4 commits into from
Closed

#615: added TYPEFLAGS constants for TYPEATTR in ITypeInfo.GetTypeAttr() #619

wants to merge 4 commits into from

Conversation

SevenOf9Sleeper
Copy link
Contributor

In addition I have made the contribution projects compileable for eclipse. I hope it is ok that way. May it be that the 'ant dist test' call is compiling the contribution projects not a bit?!

@SevenOf9Sleeper SevenOf9Sleeper mentioned this pull request Mar 14, 2016
Closed
matthiasblaesing
matthiasblaesing reviewed Mar 20, 2016
View reviewed changes
contrib/msoffice/src/com/sun/jna/platform/win32/COM/util/office/MSOfficeWordDemo.java
@@ -101,9 +101,6 @@ public void testMSWord() {
if (msWord != null) {
msWord.Quit();
}
if (null != factory) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is correct with regard to fixing the compile error, the example lacks proper COM initialization (the automatic init was removed together with the threading). (a Ole32.INSTANCE.CoInitializeEx/Ole32.INSTANCE.CoUninitialize() pair is missing)

At this point factory.disposeAll should be called to dispose all COM objects (I'm not sure how COM reacts to programm shutdown, but in any case a clean shutdown is preferred).

@matthiasblaesing
Copy link
Member

Thanks for the contribution - in addition to the line comment some more points:

  • please separate the concerns of the patches - I see two parts here: Fix the samples + add the missing constants
  • if features are added or bigger bugfixes are done, please add an entry to the CHANGES.md file

I checked the eclipse changes only superficially (after about an hour fight I finally realized I had to rename my jna checkout ...) - so while eclipse seems happy now, I'll trust you on the eclipse parts.

@twall
Copy link
Contributor

twall commented Mar 20, 2016

Maybe we need to add the “platform” target to “dist” dependencies.

There’s a separate “platform-test” target as well.

On Mar 14, 2016, at 6:58 PM, Mathias Mehrmann [email protected] wrote:

In addition I have made the contribution projects compileable for eclipse. I hope it is ok that way. May it be that the 'ant dist test' call is compiling the contribution projects not a bit?!

You can view, comment on, or merge this pull request online at:

#619

Commit Summary

• make the jna project for eclipse compileable
• remove compile error. Factory.getComThread() does not exist anymore
• remove compile error
• #615: added TYPEFLAGS constants
File Changes

• M contrib/msoffice/.classpath (4)
• M contrib/msoffice/src/com/sun/jna/platform/win32/COM/util/office/MSOfficeWordDemo.java (3)
• M contrib/native_window_msg/.classpath (2)
• M contrib/native_window_msg/src/com/sun/jna/platform/win32/Win32WindowDemo.java (8)
• M contrib/platform/.project (2)
• M contrib/platform/src/com/sun/jna/platform/win32/OaIdl.java (83)
• M contrib/w32printing/.classpath (14)
Patch Links:

• https://github.com/java-native-access/jna/pull/619.patch
• https://github.com/java-native-access/jna/pull/619.diff
—
Reply to this email directly or view it on GitHub.

@matthiasblaesing
Copy link
Member

I don't understand:

May it be that the 'ant dist test' call is compiling the contribution projects not a bit?!

Do I understand this correct, that you see tests not executed when called this way? If so I have a diagnosis:

Ant in this case executes the target "-dynamic-properties" is evaluated twice. On the second call

<condition property="cross-compile" value="true">
  <isset property="os.prefix"/>
</condition>

os.prefix is set and cross-compile will be set to true. This in turn prevents the tests from being run....

A quick fix:

# This patch file was generated by NetBeans IDE
# It uses platform neutral UTF-8 encoding and \n newlines.
--- a/build.xml
+++ b/build.xml
@@ -134,8 +134,8 @@
   <target name="compile-test-single" depends="compile-tests"/>
   <target name="compile-single" depends="compile"/>

-  <target name="-dynamic-properties">
-
+  <target name="-dynamic-properties" unless="-dynamic-properties">
+    <property name="-dynamic-properties" value="evaluated" />
     <condition property="-native" value="true">
       <not><isset property="build-native"/></not>
     </condition>

@SevenOf9Sleeper
Copy link
Contributor Author

@matthiasblaesing yes, you are right. I have mixed up corrections and features... so I have created a new pull request #620. There are the corrections for the compile problems and I have added a CoInitializeEx and CoUninitialize as you mentioned.
I understand now why the compile problems are not detected: some contrib projects haven't a build.xml. I have fixed this. Then the problems in the msoffice project come to light... :-]
When you have adopted the new pull request I will create the real feature pull request.
Unfortunately I was not able to run the native compile under windows. I have tried with an installed winbash and get the following message:
[exec] Makefile:123: Extraneous text after else' directive [exec] Makefile:128: Extraneous text afterelse' directive
[exec] Makefile:128: *** only one `else' per conditional. Stop.

@dblock
Copy link
Member

dblock commented Mar 22, 2016

I've merged #620. This whole area could use some more active maintainers :)

@twall
Copy link
Contributor

twall commented Mar 22, 2016

The Makefile expects GNU make. I’ve not tried winbash; I’ve only done builds using cygwin and only occasionally with mingw.

I would like to shake out any problems so others can more easily build under windows.

On Mar 21, 2016, at 8:57 PM, Mathias Mehrmann [email protected] wrote:

@matthiasblaesing yes, you are right. I have mixed up corrections and features... so I have created a new pull request #620. There are the corrections for the compile problems and I have added a CoInitializeEx and CoUninitialize as you mentioned.
I understand now why the compile problems are not detected: some contrib projects haven't a build.xml. I have fixed this. Then the problems in the msoffice project come to light... :-]
When you have adopted the new pull request I will create the real feature pull request.
Unfortunately I was not able to run the native compile under windows. I have tried with an installed winbash and get the following message:
[exec] Makefile:123: Extraneous text after else' directive
[exec] Makefile:128: Extraneous text afterelse' directive
[exec] Makefile:128: *** only one `else' per conditional. Stop.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub

@SevenOf9Sleeper
Copy link
Contributor Author

Ok, thanks for merging. I close now this request and make a new one for the feature (when I have successfully merged the different branches... :-] )

mstyura pushed a commit to mstyura/jna that referenced this pull request Sep 9, 2024
@normanmaurer
Use Integer.valueOf(...) to reduce object creation (java-native-acces…
2d2d488 
…s#619)

Motivation:

Integer.valueOf(...) uses a cache internally to reduce object creation.
Let's take advantage of that

Modifications:

Replace new Integer(...) calls with Integer.valueOf(...)

Result:

Less Object allocations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SevenOf9Sleeper @matthiasblaesing @twall @dblock